Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Jul 8, 2025

What do these changes do?

This PR introduces a major refactoring of the users domain in the webserver, one of the oldest and most coupled parts of the codebase. Please read this description carefully.

The primary goal is to decouple user-related subdomains — such as user_preferences, user_tokens, and user_notifications — into self-contained modules. Although conceptually tied to the user, each of these features has its own database table and business logic, and they are now implemented as independent domains.

As a result, the users domain now focuses solely on user management (i.e., logic around the users table). It continues to serve as the main entry point, with its bootstrap.py function still responsible for composing and initializing all related subdomains via setup_users().

In addition, this PR includes:

The changes are backed by comprehensive test coverage to ensure stability.

Further details include an overview of all users domain modules, a breakdown of the new structure using user_preferences/ as an example, and notes on the user_notifications refactor.

Skeleton and CSR-layer layout:

image

Here an explanation of the structure using as example user_preferences/:

  1. bootstrap.py (public)
    Initializes the domain into the aiohttp app: implements setup_*(app) function
    • Registers REST routes
    • Attaches handlers to on_startup / on_cleanup-like events
    • Initializes DB/service layer dependencies
    • Updates app state
  2. _controller/rest/*_rest.py
    REST handler module
    • Defines aiohttp route handlers (async functions)
    • Exposes endpoints for interacting with user preferences (GET/PUT, etc.)
  3. _controller/rest/_rest_exceptions.py
    REST-specific exceptions handlers:
    • Maps internal errors to HTTP responses
    • Includes well formatted and humna-readable user messages
  4. _models.py
    Domain model definitions (likely ORM or DTOs):
    • Internal data representation (Pydantic models)
  5. _repository.py
    Data access layer:
    • Encapsulates queries and persistence logic
    • Exposes methods to fetch/store preference data from DB
  6. _service.py
    Domain logic layer:
    • Implements business logic around user preferences
    • Pure functions or service classes operating over models/repositories
  7. user_preferences_service.py (public)
    Public service interface:
    • Exposes the domain’s service layer for external use
    • Acts as a façade or entry point to user_preferences from outside (e.g., from users/bootstrap.py)

Refactoring of user_notifications

  • Repository layer (user_notifications._repository.py): handles: Redis commands, data serialization/deserialization, key management. Why?
    1. Data Access Pattern: The Redis operations are pure data access/persistence operations - they're interacting with the data store (Redis) just like database operations would interact with PostgreSQL.
    2. Consistency with existing pattern: Looking at your other modules:
    - user_preferences has a repository that handles database operations via FrontendUserPreferencesRepo
    - user_tokens has a repository that handles database operations via SQLAlchemy
    - The Redis operations here are functionally equivalent - they're data persistence operations
    3. Separation of Concerns
    4. Testability: Having Redis operations in a repository makes them easier to mock and test
  • Service (user_notifications._service.py): Handle business logic (product filtering, backwards compatibility, notification creation logic), call repository methods
  • Controller (use_notification_rest.py) : Handle HTTP concerns, validation, call service methods

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv tests/unit/**/test_*user*.py

Manual exploratory testing 🚨

  • get profile
  • load/save preferences
  • load/save tokens
  • notify-users
    • share a project between two users
    • create an organization and add a user
    • share a wallet

Dev-ops

None

@pcrespov pcrespov self-assigned this Jul 8, 2025
@codecov
Copy link

codecov bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 91.77489% with 38 lines in your changes missing coverage. Please review.

Project coverage is 88.31%. Comparing base (dbd3129) to head (885ae2e).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8083      +/-   ##
==========================================
+ Coverage   87.60%   88.31%   +0.70%     
==========================================
  Files        1866     1663     -203     
  Lines       71919    66997    -4922     
  Branches     1268      938     -330     
==========================================
- Hits        63008    59169    -3839     
+ Misses       8543     7553     -990     
+ Partials      368      275      -93     
Flag Coverage Δ
integrationtests 64.36% <67.39%> (+3.77%) ⬆️
unittests 86.81% <90.90%> (-0.06%) ⬇️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.24% <100.00%> (+<0.01%) ⬆️
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration 70.17% <ø> (ø)
pkg_service_library 71.22% <100.00%> (+0.08%) ⬆️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.16% <ø> (+0.17%) ⬆️
agent 96.29% <ø> (ø)
api_server 92.84% <ø> (ø)
autoscaling 95.88% <ø> (ø)
catalog 92.58% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.79% <ø> (-0.57%) ⬇️
datcore_adapter 97.94% <ø> (ø)
director 76.77% <ø> (ø)
director_v2 91.04% <ø> (+6.41%) ⬆️
dynamic_scheduler 96.69% <ø> (ø)
dynamic_sidecar 90.09% <ø> (ø)
efs_guardian 89.65% <ø> (ø)
invitations 93.60% <ø> (ø)
payments 92.57% <ø> (ø)
resource_usage_tracker 92.52% <ø> (+0.42%) ⬆️
storage 86.72% <ø> (+0.08%) ⬆️
webclient ∅ <ø> (∅)
webserver 88.55% <91.68%> (-0.06%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbd3129...885ae2e. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pcrespov pcrespov changed the title WIP: Is64/users refactoring Major Refactor: Isolate User Subdomains & Modernize Internal Structure Jul 9, 2025
@pcrespov pcrespov changed the title Major Refactor: Isolate User Subdomains & Modernize Internal Structure ♻️ Major Refactor: Isolate webserver's user Subdomains & Modernize Internal Structure (🚨) Jul 9, 2025
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Jul 9, 2025
@pcrespov pcrespov marked this pull request as ready for review July 9, 2025 17:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the users domain by isolating its subdomains (user_preferences, user_notifications, user_tokens) into independent modules, updating import paths throughout the codebase and tests, and modernizing bootstrap registration.

  • Updated import paths in unit tests to reference new modules (users_service, user_preferences, user_notifications) instead of legacy users.api or _common.
  • Replaced direct calls to internal user APIs with public service facades (users_service, user_preferences_service, UserNotificationsRepository).
  • Updated users/plugin.py to compose subdomain features via dedicated bootstrappers, and removed legacy REST exception handlers in favor of consolidated _rest_exceptions.

Reviewed Changes

Copilot reviewed 91 out of 106 changed files in this pull request and generated no comments.

File Description
services/web/server/tests/** Adjusted test imports to the new module structure
services/web/server/src//users/ Replaced users.api imports with users_service or new models
services/web/server/src/**/user_* Introduced user_preferences, user_notifications, user_tokens modules with clear bootstrap and service layers
packages/service-library/** Added ensure_single_setup decorator to observer registry
Comments suppressed due to low confidence (1)

packages/service-library/src/servicelib/aiohttp/observer.py:24

  • The variable 'log' is not defined in this module, leading to a NameError at runtime. It should reference a valid logger (e.g., import or define _logger) or use a correct logger variable.
@ensure_single_setup(__name__, logger=log)

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀
Let me know when you want check the notifications 👍

@pcrespov pcrespov requested a review from GitHK July 10, 2025 09:23
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 11, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort

@pcrespov pcrespov merged commit 23ec8aa into ITISFoundation:master Jul 11, 2025
96 checks passed
@pcrespov pcrespov deleted the is64/users-refactoring branch July 11, 2025 12:39
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintenance: Refactor in users domain in webserver

7 participants